-
Notifications
You must be signed in to change notification settings - Fork 136
fix(ci/package_on_device): do not conflict gh release upload archive filename with packages.yml
#2044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
95e7d00 to
d9cce5e
Compare
d9cce5e to
4b97c8a
Compare
…ve` filename with `packages.yml`
e3b6f11 to
50c80e0
Compare
…t-packages.sh` to TUR URL
50c80e0 to
cb8b820
Compare
| echo "$archive uploaded" | ||
| tur_on_device_archive="${archive//.tar/-tur-on-device.tar}" | ||
| mv "$archive" "$tur_on_device_archive" | ||
| gh release upload -R https://github.com/termux-user-repository/tur "0.1" "$tur_on_device_archive" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@licy183 do you think this will work?
I don't want to separate packages of tur and tur-on-device folders into separate PRs,
instead, I want to try to fix it permanently so that the problem never occurs again.
However, unfortunately, I don't understand exactly what causes the problem, only that there was an error with this file conflicting because both workflows ran in 1 PR.
Can the code which takes the unprocessed_debfile https://github.com/termux-user-repository/tur/releases/tag/0.1 and sends it to https://tur.kcubeterm.com automatically handle the filename sometimes being something different, like debs-{aarch64,arm,i686,x86_64}-${{ github.sha }}-tur-on-device.tar, or will that not work and it has to be exactly debs-{aarch64,arm,i686,x86_64}-${{ github.sha }}.tar every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work correctly. Currently, the scripts in the dists repo do not validate filenames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work as the CI reuse can't pick up the files uploaded by the on-device worflow. The best way to avoid this is avoid building tur-on-device packages and tur packages in the same MR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh no... actually, I did not ever know whether CI reuse would actually work with tur-on-device or not.
The reason why I pasted the lines for it in this commit 70cc0df was because if I didn't do that, a build error occurred because of a variable or setting of some kind missing that prevented building all tur-on-device packages at all after the merge of CI reuse in the main repository.
I don't know how to fix it, so it is necessary to disable CI reuse for all PRs which modify tur-on-device packages.
I am trying again here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, CI reuse can work on tur-on-device.
The situation where it doesn't work properly is when the packages from tur and tur-on-device are modified in the same PR.
I think there is no need to revert it. I've uploaded them manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, CI reuse can work on tur-on-device.
I thought that it didn't because I saw this log, in an earlier PR where I modified only tur-on-device packages:
https://github.com/termux-user-repository/tur/actions/runs/19011632972/job/54293393376
Do you know of an example where it worked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should work, but I actually don't know whether it works. The most important thing is to avoid modifying build.shs in both tur and tur-on-device in the same PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to fix it so that modifying build.sh in both tur and tur-on-device in the same PR is possible,
because I do not want anyone else to ever encounter the same problem, and the draft PR I opened can do that
However, it does it by disabling the CI reuse for tur-on-device PRs.
I will try to figure out any possible way to do it without disabling the CI reuse.
I believe that part of the problem is that the CI reuse has never worked properly for tur-on-device packages (which can be seen in the log above), and fixing that might be where I can start, I don't know how to fix it yet but I will try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -0,0 +1,15 @@ | |||
| 'git reset HEAD~' is TEMPORARY - work around bug in lint-packages.sh | |||
| will discuss with Tomjo2000 and fix upstream later | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have discussed now, and we think that it will get fixed by today's update to this:
this patch can be rebased to remove git reset HEAD~ after that PR is merged.

Retry revbump({tur,tur-on-device}): for
imath3.2.2,openvdb13.0.0 #2037Attempt to fix [Bug]: blender4 needs rebuild #2043